Skip to content

SimpleStream improvements and logging#1123

Closed
jameson-dev wants to merge 5 commits into
TrunkRecorder:masterfrom
jameson-dev:upstream/simplestream-improvements
Closed

SimpleStream improvements and logging#1123
jameson-dev wants to merge 5 commits into
TrunkRecorder:masterfrom
jameson-dev:upstream/simplestream-improvements

Conversation

@jameson-dev

Copy link
Copy Markdown
Contributor

Description for changes have been added to each commit. Happy for criticism on these changes 😄

Here's a brief breakdown on the changes:

  • Move streams, my_tcp_io_service, and max_tcp_index from file-scope globals into the Simple_Stream class as private members, preventing shared state between instances

  • Fix BOOST_FOREACH loops to iterate by const reference rather than copying the struct by value on every audio packet

  • Wrap TCP send() calls in try/catch to prevent a dropped connection from crashing the entire trunk-recorder process

  • Log a warning when UDP send_to() fails silently so network errors are visible in logs

  • Replace ip::address::from_string() with Boost.Asio resolvers for both UDP and TCP connections, allowing hostnames to be used in the config in addition to literal IP addresses

Note: I haven't actually tested these changes at all not having a TR setup to test with at the moment, but I'm rather confident of the changes. I would definitely test these changes first.

File-scope globals were shared across every instance of the class. If more than one SimpleStream instance were ever loaded, they would silently share the same stream list and TCP I/O service, likely causing data corruption and race conditions.
Copying a struct with a raw pointer wont manage the socket lifetime, and two copies of the same pointer could interact badly. It also needlessly copies two std::string members and a UDP endpoint on every audio packet, which is a hot path. The const auto & form avoids all copies entirely. The start() and stop() loops already used auto& correctly, so this change just introduces consistency
Dropped TCP connections throws boots::system::system_error, which would terminate the TR process. It  should now log errors and continue. For UDP, there was no logging for errors to know when packets are dropped
from_string() only accepts literal IP addresses, entering a hostname would throw an error and crash TR with no explanation.
@Dygear

Dygear commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

I really wanted someone to make udp://hostname.tld:8600 or tcp://hostname.tld:8600 possible. We got the hostname part, so that's cool. That way it's a much tighter config statement.

Adds support for a url field in the simplestream config.  Can now be defined as udp://hostname:port or tcp://hostname:port instead of
separate address, port, and useTCP fields.

Added deprecation warning log and updated documentation to reflect changes and deprecation intent
@jameson-dev

Copy link
Copy Markdown
Contributor Author

Something like this 318c58c, @Dygear?

@Dygear

Dygear commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Exactly!

@robotastic

Copy link
Copy Markdown
Collaborator

Is anyone able to give these a test? I don't have SimpleStream setup

@Dygear

Dygear commented May 1, 2026

Copy link
Copy Markdown
Contributor

I'll put it in on my next update cycle.

@robotastic robotastic mentioned this pull request May 30, 2026
robotastic added a commit that referenced this pull request May 30, 2026
* Move globals into respective class

File-scope globals were shared across every instance of the class. If more than one SimpleStream instance were ever loaded, they would silently share the same stream list and TCP I/O service, likely causing data corruption and race conditions.

* Use const auto & in BOOST_FOREACH stream loops

Copying a struct with a raw pointer wont manage the socket lifetime, and two copies of the same pointer could interact badly. It also needlessly copies two std::string members and a UDP endpoint on every audio packet, which is a hot path. The const auto & form avoids all copies entirely. The start() and stop() loops already used auto& correctly, so this change just introduces consistency

* Wrap all TCP calls with try/catch and add error logging

Dropped TCP connections throws boots::system::system_error, which would terminate the TR process. It  should now log errors and continue. For UDP, there was no logging for errors to know when packets are dropped

* Use resolver for hostname support and add error logging

from_string() only accepts literal IP addresses, entering a hostname would throw an error and crash TR with no explanation.

* Implement URL-based configuration

Adds support for a url field in the simplestream config.  Can now be defined as udp://hostname:port or tcp://hostname:port instead of
separate address, port, and useTCP fields.

Added deprecation warning log and updated documentation to reflect changes and deprecation intent

* Merge master into PR #1123 (SimpleStream improvements)

Resolves conflicts in plugins/simplestream/simplestream.cc between the PR's
class-encapsulation/resolver work and master's ASIO modernization (#1129).

PR intent preserved:
- streams/io_context/max_tcp_index moved from file-scope globals into
  Simple_Stream class members
- BOOST_FOREACH loops use const auto& to avoid copying stream_t
- TCP send wrapped in try/catch; UDP send_to errors logged
- Hostname resolution support via resolver (now using the modern
  resolve(host, service) overload instead of the deprecated
  resolver::query / resolver::iterator API, since master moved to io_context)
- url-based config (udp://host:port / tcp://host:port) with deprecation
  warning for address/port/useTCP

Master changes preserved:
- io_service -> io_context rename (#1129) applied to both my_io_context
  and the moved my_tcp_io_context
- json_length scope fix (#1107) already absorbed via the PR base

Docs auto-merged: docs/CONFIGURE.md keeps both master's outputRawAudio
additions and the PR's url field documentation.


---------

Co-authored-by: jameson-dev <178156579+jameson-dev@users.noreply.github.com>
@robotastic

Copy link
Copy Markdown
Collaborator

I resolved the conflicts and merged it in!

@robotastic robotastic closed this May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants